Skip to content

PR-C: implement locked review rulings (S-01, S-03, S-05, S-09, S-11, S-12, S-13) - #14

Merged
VSP-0 merged 3 commits into
mainfrom
pr-c-rulings
Aug 30, 2026
Merged

PR-C: implement locked review rulings (S-01, S-03, S-05, S-09, S-11, S-12, S-13)#14
VSP-0 merged 3 commits into
mainfrom
pr-c-rulings

Conversation

@VSP-0

@VSP-0 VSP-0 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

PR-C: implement the locked security-review rulings

Closes out the remaining findings from @ibnu76's review of 58971c05, per the
rulings locked in SESSION-STATE. Builds on merged main (aa7d477 = PR-A #12 +
PR-D #13). Pre-genesis, so storage-layout and ABI changes are free.

Rulings implemented

S-01 (Low) — bucketIndexRay == 0 sentinel collision → honest init (Option 1).
The pooled tail bucket used 0 as a lazy "means RAY" sentinel. Sustained losses
decay the index toward zero; the moment a settlement wrote exactly 0, the next
read resurrected every wiped bucket at face value — insolvency (~1400 days at
deployed rates, ~250 at the 5e18 policy cap; S01DeployParams has the sweep).
Now: _bucketAdd initializes the index to RAY explicitly on first entry (safe:
a floor-guaranteed nonzero index means 0 ⇔ no member has ever entered, zero scaled shares), _bucketIndex returns storage verbatim, and _settleBucket
floors the index at 1 wei — a fully wiped bucket is dust-dead, exits stay live
via the full-exit branch, and no value can be conjured back. _projectBucket
was rewritten to project through the index with identical operations and the
same floor
, so view == materialized to the wei even in the wipe regime
(a V.8 fidelity improvement over the old live-based formula).

S-03 (Low) — sMax tracker, layered per the ruling.
(i) Never snap down. The old code snapped sMax to the tracked leader the
instant the leader shrank — a 1-wei dust post could drag sMax to dust and
inflate every post's participation factor to the clamp. Decay (10%/epoch,
30-epoch catch-up cap) is now the sole descent, floored at the tracked
leader. (ii) Permissionless refreshSMax(postId). The irreducible poke for
lazy-accrual dormant posts: settles the post if an epoch has passed, then feeds
its true stored total to the tracker. It can only make sMax more honest, so
it is open to anyone; the ops worker will poke the largest posts each epoch
(companion app-repo patch). Emits SMaxRefreshed. (iii) Tracker widened
3 → 10
(TRACKED_POSTS); getTopPosts() still returns the top three pairs
for ABI stability.

S-04 (closed, ratified) + I.4 re-documented. ECONOMIC_INVARIANTS.md I.4
is rewritten to be true-about-this-mechanism: sMax ≥ every tracked total after
update; deviations from untracked dormant posts are closeable by anyone via the
poke; participation clamps at 1.0 bound the pre-poke worst case at the rMax
ceiling. The cross-post coupling through global sMax is ratified as intended
design in one sentence (code comment at the participation site + I.4).

S-05 (Info) — false rMax < RAY comments. Both bucket-math sites now state
the true bound: rMax scales with elapsed epochs, rBase can exceed RAY under
dormancy, and the gRay >= RAY wipe branch is live (correct total-loss
floor), not dead as the old comment claimed.

S-09 (Info) — decay-rate docstring. 9e17 (10%/day) was always the deployed
constant and is correct as the backstop; the docstring claiming 0.5%/day was
the bug. Docstring fixed; Deploy.s.sol now sets the value explicitly on the
dev path and require-pins it on every path, so drift fails the deploy loudly.

S-11 (Info) — StakeLot.entryEpoch removed. Stored, never read by any
settlement path. Struct field, write site, and the view tuple slot are gone:
getUserLotInfo returns (amount, weightedPosition, sideTotal, positionWeight). Interface updated; all in-tree destructurings mechanically
migrated (36 sites).

S-12 (Info) — _rescalePositions removed. Positions are recomputed as
midpoints (< total) after every mutation and settlement, so the rescale body
was unreachable outside the neutral branch, where it was a no-op. Function,
both call sites, and the PositionsRescaled event are gone; the _applyEpoch
clamp remains as the safety net.

S-13 (Info) — dead ZeroAddress branch in acceptGovernance removed.
When pending is address(0) no caller can equal it, so NotPendingGovernance
always fired first. Behavior identical; regression test pins the revert.

Behavior change to be aware of (intended)

Under never-snap-down, an exited whale's sMax peak no longer vanishes
instantly — it decays (bounded, ≤ ~90 epochs to burn 4 orders of magnitude,
faster with per-epoch pokes), transiently suppressing smaller posts' rates.
That trade is the point: instant snap-down was the manipulation lever.
S04SpecTestPoC::test_Q2 now documents the full arc: suppression exists →
pokes burn the peak down → rate recovers to the never-whaled rate within 2%.

Tests

  • Reviewer PoCs for fixed findings converted to regression form (insolvency →
    solvency asserts for S-01; drag/snap-down asserts inverted for S-03 with the
    poke closing the documented pre-poke gap; S-11 rewritten as a 4-tuple
    queue-position regression). Untouched PoCs remain as evidence.
  • New test/PrCRegressions.t.sol: 7 targeted regressions (never-snap-down;
    exact decay curve + leader floor; permissionless poke honesty; 10-slot
    tracker; wiped-bucket solvency across two settlements incl. member exit;
    fresh-bucket re-init; S-13 revert).
  • Gate: 332 passed / 0 failed across 55 suites, reproduced twice — once in
    the working sandbox, once on a pristine reconstruction of merged main with
    only the shipped editors applied.
  • Post-patch src/StakeEngine.sol sha256:
    4c6f8dc99a10a0dd4ec2d47ace930a123a3f13fb48d3eeedeea62add777f17a9.

Toolchain note (worth reading — cost us a phantom-bug hunt)

solc 0.8.33 under this profile caches in-frame block.timestamp after the
first read
, so chains of vm.warp(block.timestamp + X) inside one test frame
collapse to a single warp (the warp applies to the env; the test's re-read is
stale — engine/handler frames always read fresh, so contracts and PR-D's
invariant handler are unaffected). New tests use vm.getBlockTimestamp() for
warp arithmetic. Existing PoCs are left as authored: they are empirically
anchored on this exact toolchain, and their _fresh() absolute-warp pattern
re-anchors each scenario correctly.

Follow-ups

  • Fresh Fuji redeploy required (storage + ABI changed): sequence with the
    Track B mock deploy; app/deployments/fuji.json addresses rotate and the app
    repo needs the 4-tuple getUserLotInfo ABI.
  • Companion app-repo patch: ops-worker keeper poking refreshSMax each epoch
    for the largest posts.
  • Slither: code removal may strand stale baseline fingerprints; if the CI gate
    complains, re-run with --update (expected shrink only, no new findings).

VSP-0 added 3 commits August 30, 2026 16:03
…S-12, S-13)

S-01: honest bucket-index init; settlement floors index at 1 wei — the
0==RAY lazy sentinel that resurrected wiped buckets is gone (root cause).
S-03: sMax never snaps down; decay is the sole descent, floored at the
tracked leader; permissionless refreshSMax(postId) closes lazy-accrual
deviations; tracker widened to TRACKED_POSTS=10. I.4 re-documented as
true-about-mechanism; S-04 global coupling ratified as intended.
S-05: false rMax<RAY comments corrected at both bucket sites.
S-09: 9e17 (10%/day) is correct; docstring fixed; Deploy.s.sol pins it.
S-11: StakeLot.entryEpoch removed; getUserLotInfo now a 4-tuple.
S-12: _rescalePositions + PositionsRescaled removed (dead code).
S-13: unreachable ZeroAddress branch removed from acceptGovernance.

Reviewer PoCs for the fixed findings converted to regression form;
S04SpecTestPoC Q2 documents the changed (intended) exited-whale arc.
New test/PrCRegressions.t.sol: 7 targeted regressions. Suite 332/0.

Pre-genesis: storage layout and ABI changes are free; fresh Fuji
redeploy required (sequenced with Track B mock deploy).
…ntryEpoch removal; pre-genesis, fresh redeploy planned)
…th — nonReentrant + own-token, same class as stake/withdraw; _projectBucket divide-before-multiply — intentional truncation-order mirror of _settleBucket for V.8) + prune fingerprints of removed code
@VSP-0
VSP-0 merged commit 148a8f3 into main Aug 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant